Note that get_position is used by many other functions too. It wasn't me who wrote this line, but I could imagine that .begin() was used intentionally, instead of .b (caret position), even though the latter seems more intuitive. For example, if the caret is in the middle of a word, and you press Ctrl+D or use "expand selection" (Ctrl+Shift+A), then in both cases the caret will move to the end of the word (or block etc.). But this means that the caret position now actually points to the next character, which is likely a whitespace or a ( for example. This might be the reason why the begin of a selection was preferred over the actual caret position in this case.

I have tested with a few language servers, and all of them were smart enough to respond with the correct result even if the point (caret) is at the very end (i.e. actually behind) an identifier name. Based on this, I would assume that the proposed change should be acceptable, and I agree that it would feel more natural to always use the caret position. But I can't rule out whether there are any unintended side effects.

It make sense to use the caret position.

The previous logic was added in June 2020, and get_position was used in the following:
LspOpenLinkCommand
LspGotoCommand
LspHierarchyCommand
LspSymbolReferencesCommand
LspSymbolRenameCommand
LspExpandSelectionCommand
LspTextCommand

But I can't rule out whether there are any unintended side effects.

It is not easy to predict if it causes unintended side effects or not,
the best is to test all affected commands to at least confirm that it works as before.

I have used this code for the past 6 months or so, haven't noticed any changes. I've tested the commands you mentioned above, and I have not noticed any changed behavior. Is it possible to move forward with this? If not, would you be okay with a PR that makes this behavior optional, configurable with a boolean in the settings?

I'm more for the change you proposed.
I want to invest some time into testing it before merging.

I'll try to find the time next week.

✅ Deploy Preview for sublime-lsp ready!

Name	Link
🔨 Latest commit	df47745
🔍 Latest deploy log	https://app.netlify.com/sites/sublime-lsp/deploys/6628c13563250b000808c839
😎 Deploy Preview	https://deploy-preview-2440--sublime-lsp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

I am starting to test this.

@trbjo can you give me more detailed steps on how to reproduce this case(or a short recording of your screen)?

I cannot really reproduce the behavior with the description from here:

I use expand_to_... to navigate, and it's annoying that when I land on a symbol, I can't see the popup before the selection is empty again.

Before I would merge this, because the change makes sense.
But I would really try the usecase that you described here.

I use expand_to_... to navigate, and it's annoying that when I land on a symbol, I can't see the popup before the selection is empty again.

Like I said,
I could not really replicate this behavior that you decribled with the main branch.
Can you add some video recording of your case?

I haven't discovered that this PR breaks anything.
I also didn't discover/conclude what exact use case this PR solves.

So if anyone has an example that this PR solves, feel free to provide it.